update e2e tests block numbers#451
Merged
Merged
Conversation
Contributor
WalkthroughThe PR rewires the ChangesE2E Test Infrastructure Refresh
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.43.0)test/e2e/e2e.test.jsThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/e2e/data.js (1)
114-140:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep positional arrays aligned after token removal.
After commenting out WLTH, the Base config now has 3 active tokens/holders but 4 deposit entries. This silent drift makes token↔deposit mapping fragile and can cause wrong amounts if tokens are toggled again.
Suggested fix
- ["1", "10000", "10000", "10000"], + ["1", "10000", "10000"],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/data.js` around lines 114 - 140, The Base configuration has a mismatch between the number of active Token objects and the entries in the holders and deposit amounts arrays. After commenting out the WLTH token, there are now 2 active tokens but the holders array and deposit amounts array still contain 4 entries instead of 2-3. To fix this, remove or comment out the holder address and corresponding deposit amount entry that was aligned with the commented-out WLTH token from both the holders array (around "0xb2cc224c1c9feE385f8ad6a55b4d94E92359DC59" and its peer) and the deposit amounts array (around ["1", "10000", "10000", "10000"]) to ensure all three positional arrays have matching lengths and proper token-to-holder-to-amount mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/standard-test.yaml:
- Around line 68-70: The inline unquoted secret assignment in the run command on
line 68 using TEST_${{ matrix.chain }}_RPC=${{ secrets[env.RPC] }} can break if
the RPC URL contains shell-significant characters. Remove this inline secret
expansion from the run command and instead use the DEFAULT_RPC environment
variable that is already being injected in the env section on line 70. Export
the dynamic variable (TEST_<chain>_RPC) from DEFAULT_RPC within the run command
itself to safely handle special characters in the secret value.
In `@test/e2e/e2e.test.js`:
- Around line 217-221: The hex funding value "0x3635C9ADC5DEA00000" is
duplicated multiple times throughout the test file in both hardhat_setBalance
calls and bot.BALANCE assignments, which can lead to inconsistency if one value
is updated and the other is not. Define a constant at the top of the test file
to hold this hex value, then replace all instances of this hardcoded string with
references to that constant in both the
network.provider.send("hardhat_setBalance", [...]) call and the bot.BALANCE
assignment using ethers.BigNumber.from().
---
Outside diff comments:
In `@test/e2e/data.js`:
- Around line 114-140: The Base configuration has a mismatch between the number
of active Token objects and the entries in the holders and deposit amounts
arrays. After commenting out the WLTH token, there are now 2 active tokens but
the holders array and deposit amounts array still contain 4 entries instead of
2-3. To fix this, remove or comment out the holder address and corresponding
deposit amount entry that was aligned with the commented-out WLTH token from
both the holders array (around "0xb2cc224c1c9feE385f8ad6a55b4d94E92359DC59" and
its peer) and the deposit amounts array (around ["1", "10000", "10000",
"10000"]) to ensure all three positional arrays have matching lengths and proper
token-to-holder-to-amount mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fecde88c-803f-4dbe-bcd6-a4cab804d49e
📒 Files selected for processing (4)
.github/workflows/standard-test.yamlhardhat.config.tstest/e2e/data.jstest/e2e/e2e.test.js
Contributor
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. SIZE=M |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Tests
Chores